Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Roadmap JSON in Storybook #2297

Merged
merged 22 commits into from
Aug 6, 2024
Merged

Conversation

benlister-okta
Copy link
Contributor

DES-6079

Summary

Update storybook roadmap page JSON with latest from July 2024.

@benlister-okta benlister-okta requested a review from a team as a code owner July 23, 2024 18:30
@benlister-okta benlister-okta changed the base branch from main to bl_read_only_forms_OKTA-731946 August 1, 2024 20:54
@benlister-okta benlister-okta changed the base branch from bl_read_only_forms_OKTA-731946 to main August 1, 2024 20:54
Comment on lines +44 to +60
const getTooltipText = (
defineValue: string,
designValue: string,
developValue: string,
): string => {
let text = "";
if (defineValue === "In progress") {
text += "Project definition in progress";
}
if (["Complete", "In progress"].includes(designValue)) {
text += (text ? " " : "") + "Design: " + designValue;
}
if (["Complete", "In progress"].includes(developValue)) {
text += (text ? " " : "") + "Develop: " + developValue;
}
return text.trim();
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is just the roadmap page, these are optional changes.

In general, I prefer not overloading the + operator and especially not doing mutations. I would instead write this with .concat() and return to avoid mutations.

If it works and looks right, I'm not gonna say much for this PR; I just don't want bad patterns in the codebase in general. I also don't wanna waste time on code for doc pages unless it's going to be too fragile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Since this is pre-existing and internal only (roadmap page), I am going to tackle this later but I just created a ticket for myself for those changes. There are some other lingering things to explore in the roadmap page as well (the root cause of why it was infinitely refreshing with pagination, search, and filters enabled) so I may work on it when I do that work.

@oktapp-aperture-okta oktapp-aperture-okta bot merged commit e2f67eb into main Aug 6, 2024
1 check passed
@oktapp-aperture-okta oktapp-aperture-okta bot deleted the bl_roadmap_update_jul24 branch August 6, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants